Skip to content

refactor: unify android runtime c++ layout#45

Open
DjDeveloperr wants to merge 14 commits into
codex/import-napi-android-history-lfsfrom
codex/plan-android-cpp-refactor
Open

refactor: unify android runtime c++ layout#45
DjDeveloperr wants to merge 14 commits into
codex/import-napi-android-history-lfsfrom
codex/plan-android-cpp-refactor

Conversation

@DjDeveloperr

@DjDeveloperr DjDeveloperr commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • keep imported Android Java/project sources under platforms/android, while sharing Android C++ through NativeScript/runtime/android, NativeScript/napi/android, and NativeScript/ffi/jni/napi
  • move existing Apple FFI under NativeScript/ffi/objc and update generated-output/test wiring for the unified layout
  • keep shared Node-API headers under NativeScript/napi/common, while preserving the drifted Android engine adapters under NativeScript/napi/android for parity
  • collapse Android HERMES and SHERMES onto the same Static Hermes (shermes) header/library surface already used by Apple; SHERMES remains only a compatibility engine selector
  • remove the earlier Android engine-specific runtime test gates from the review stack so Android test assets stay at imported upstream parity
  • harden JNI Node-API constructor/object fallback behavior for Static Hermes and preserve ObjC bridge ownership/expandos across Apple engines

Review Notes

  • NativeScript/napi/android does not mean the engine adapters are intended to stay Android-only forever. The common Node-API headers are shared now, but the engine implementations have real drift across Android and Apple, so this PR keeps the imported Android copies intact while moving them out of platforms/android.
  • 214ca63e is rename-only for Android engine copies: 1,239 renames, 0 insertions, 0 deletions. The old Android napi/common removal is in the build-wiring commit, and the old Android Hermes header-surface removal is in the Static Hermes commit.
  • 690a8a6f now keeps Android HERMES/SHERMES on the imported Android Hermes sources while wiring the rest of the Android C++ layout. c4246a68 owns the actual Static Hermes unification.
  • Android module-loader pending exceptions still use the imported clear-and-wrap behavior; the earlier return-null drift was removed during the sanity pass.
  • The old fix(android): align hermes napi adapters commit is gone as a standalone review step. Its temporary include_shermes/split-header work is folded into the Static Hermes unification commit below.
  • The old test-gating/revert pair is gone from this branch. Any remaining xit/xdescribe usages are pre-existing upstream disabled specs or Jasmine infrastructure, not new engine-specific gates from this PR.

Commit Review Order

  1. Mechanical layout moves:
    • f87402a7 refactor: move objc ffi under platform namespace
    • dec31505 refactor: move android jni napi ffi into shared tree
    • 20bcb287 refactor: move android runtime sources into shared tree
    • 214ca63e refactor(android): move napi engine copies into shared tree
  2. Build wiring and generated path cleanup:
    • 690a8a6f refactor: wire android build to unified runtime layout
    • 8ee9d157 fix(apple): update objc ffi generated paths
  3. Static Hermes and Android parity fixes:
    • c4246a68 refactor(android): unify static hermes backend
    • e1f8982b fix(android): preserve unified runtime behavior
    • 793d549d fix(android): stabilize runtime test harness
    • 4c4114f0 fix: clean up runtime refactor paths
    • 1f659aca fix(jni): handle static hermes constructor receivers
  4. Cross-engine runtime fixes and iOS runner hygiene:
    • 9872c1bb fix(objc): preserve bridge object ownership
    • 732c335d fix(quickjs): clear weak refs before forced gc
    • 64966d56 test(ios): skip app main in scripted runner

Validation

  • Latest sanity-pass checks after rewriting the review stack:
    • git diff --check
    • npm run check:ffi-boundaries
    • python3 -m py_compile metadata-generator/build-step-metadata-generator.py
    • final stale-path scan for old Android C++/FFI/Hermes paths: no hits
    • 690a8a6f boundary check: Android Hermes paths still point at imported src/main/cpp/napi/hermes
    • c4246a68 boundary check: old shermes/fbjni references are gone
    • git show --find-renames=99% --name-status --format= 214ca63e | awk '$1=="D"{print}' | wc -l: 0
    • git show --shortstat --oneline 214ca63e: 1239 files changed, 0 insertions(+), 0 deletions(-)
  • Earlier Android all-engine matrix, Pixel_9_Pro_API_36 emulator, JDK 17, logs in /tmp/ns-runtime-android-engine-matrix-final-20260608-235950:
    • V8-10: 443 specs, 0 failures, 12 skipped, 16 disabled
    • V8-11: 443 specs, 0 failures, 12 skipped, 16 disabled
    • V8-13: 443 specs, 0 failures, 12 skipped, 16 disabled
    • QUICKJS: 443 specs, 0 failures, 12 skipped, 16 disabled
    • QUICKJS_NG: 443 specs, 0 failures, 12 skipped, 16 disabled
    • PRIMJS: 443 specs, 0 failures, 12 skipped, 16 disabled on rerun; initial run hit one TNS Workers > Should keep the worker alive after error timeout, logs in /tmp/ns-runtime-android-primjs-rerun-20260609-001236
    • JSC: 442 specs, 0 failures, 36 skipped, 46 disabled
    • HERMES: 442 specs, 0 failures, 52 skipped, 43 disabled
    • SHERMES: 442 specs, 0 failures, 52 skipped, 43 disabled
  • Android parity spot-checks against standalone NativeScript/napi-android:
    • V8-10: 443 specs, 0 failures, 12 skipped, 16 disabled
    • PRIMJS: 443 specs, 0 failures, 12 skipped, 16 disabled
    • removing the PR's temporary engine-specific worker gates made both this branch and standalone napi-android JSC crash at TNS Workers > Should throw exception when no parameter is passed with NativeScriptException, with no XML result; parity logs in /tmp/ns-runtime-no-fd62-android-matrix-20260609-002813 and /tmp/napi-android-standalone-jsc-parity-20260609-003053
  • Earlier iOS simulator direct-engine matrix, logs in /tmp/ns-runtime-apple-matrix-final7:
    • v8: 713 specs, 0 failures, 13 skipped
    • hermes: 713 specs, 0 failures, 11 skipped
    • quickjs: 713 specs, 0 failures, 13 skipped
    • jsc: 713 specs, 0 failures, 13 skipped
  • Earlier macOS direct-engine matrix, logs in /tmp/ns-runtime-apple-matrix-final7:
    • v8: 713 specs, 0 failures, 10 skipped
    • hermes: 713 specs, 0 failures, 8 skipped
    • quickjs: 713 specs, 0 failures, 10 skipped
    • jsc: 713 specs, 0 failures, 10 skipped
  • Android-only engine probe on Apple CMake: primjs, quickjs_ng, v8-10, v8-11, and v8-13 are still unsupported/unwired on Apple in this PR
  • Earlier local Android CMake spot-checks after reverting the temporary test gates:
    • ./gradlew ':runtime:buildCMakeDebug[x86]' -Pengine=QUICKJS_NG -PonlyX86: :runtime:buildCMakeDebug[x86] completed; the Gradle invocation then failed in the :app:buildMetadata finalizer path under local JDK 25 (:android-metadata-generator:compileKotlin rejects 25.0.2, and :dts-generator:compileJava fails on existing -Werror warnings)
    • ./gradlew ':runtime:buildCMakeDebug[x86]' -Pengine=QUICKJS -PonlyX86: :runtime:buildCMakeDebug[x86] and :runtime:buildCMakeRelWithDebInfo[x86] completed; same unrelated metadata finalizer failures under JDK 25

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 07c4d3e8-57ac-4b6b-a9ab-3f30a6765cfc

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/plan-android-cpp-refactor

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@DjDeveloperr DjDeveloperr marked this pull request as ready for review June 6, 2026 19:10
@ammarahm-ed ammarahm-ed self-requested a review June 8, 2026 08:40
@DjDeveloperr DjDeveloperr force-pushed the codex/plan-android-cpp-refactor branch from 6d556df to 5058e0d Compare June 17, 2026 22:11
Moves the imported Android engine-specific Node-API adapters out of the Android test-app C++ tree while preserving them as Android copies. The shared Node-API headers live under NativeScript/napi/common; merging the drifted engine adapters is intentionally left out of this mechanical move.
@DjDeveloperr DjDeveloperr force-pushed the codex/plan-android-cpp-refactor branch from 5058e0d to 4f9c5df Compare June 17, 2026 22:41
@DjDeveloperr DjDeveloperr force-pushed the codex/plan-android-cpp-refactor branch from 4f9c5df to 64966d5 Compare June 17, 2026 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant